Connect My Computer: Join cluster #29479

gzdunek · 2023-07-24T17:01:58Z

Contributes to #27881

This PR adds the final step of the setup process - running the agent and joining the cluster.
More info in the RFD https://github.com/gravitational/teleport/blob/master/rfd/0133-connect-my-computer.md#joining-the-cluster.

Some notes on the changes:

In the create config file request, I sent profileName to the function in the main process. It was the only thing that necessary for the function. However, when launching the agent, it is more convenient to relay on RootClusterUri, especially when sending updates to the renderer. I decided to switch to RootClusterUri everywhere, to be consistent.
I was not sure how to show errors from the spawned process in the setup document. I decided for replacing the attempt value with the error if it happens, so the error in the UI is shown closely to the place where it come from. However, replacing attempts is quite complicated, so maybe we can just show an error notification.

ravicious

I haven't run it yet, but the general idea behind the implementation seems good. As I mentioned in the last comment, let's set up a call to talk about some details related to async orchestration.

web/packages/teleterm/src/mainProcess/agentRunner.ts

...m/src/ui/ConnectMyComputer/DocumentConnectMyComputerSetup/DocumentConnectMyComputerSetup.tsx

web/packages/teleterm/src/ui/services/connectMyComputer/connectMyComputerService.ts

web/packages/teleterm/src/mainProcess/agentRunner.test.ts

...m/src/ui/ConnectMyComputer/DocumentConnectMyComputerSetup/DocumentConnectMyComputerSetup.tsx

…d on `runtimeSettings` and `rootClusterUri`. Pass `rootClusterUri` instead of `profileName` `createAgentFile`

ravicious

I'm going to review changes to the document on Monday.

...m/src/ui/ConnectMyComputer/DocumentConnectMyComputerSetup/DocumentConnectMyComputerSetup.tsx

web/packages/teleterm/src/mainProcess/agentRunner.test.ts

web/packages/teleterm/src/mainProcess/processKiller/processKiller.test.ts

web/packages/teleterm/src/mainProcess/processKiller/processKiller.ts

ravicious

I think there's a type error related to types, could you take a look at it? I didn't manage to find time to actually run the code anyway, will continue tomorrow.

web/packages/teleterm/src/ui/ConnectMyComputer/connectMyComputerContext.tsx

web/packages/teleterm/src/mainProcess/agentRunner/agentRunner.ts

# Conflicts: # web/packages/teleterm/src/mainProcess/fixtures/mocks.ts # web/packages/teleterm/src/mainProcess/runtimeSettings.ts # web/packages/teleterm/src/mainProcess/types.ts # web/packages/teleterm/src/ui/Documents/DocumentsRenderer.tsx

web/packages/teleterm/src/mainProcess/agentRunner/agentRunner.ts

web/packages/teleterm/src/mainProcess/terminateWithTimeout/terminateWithTimeout.ts

web/packages/teleterm/src/mainProcess/agentRunner/agentRunner.ts

...m/src/ui/ConnectMyComputer/DocumentConnectMyComputerSetup/DocumentConnectMyComputerSetup.tsx

web/packages/teleterm/src/ui/ConnectMyComputer/connectMyComputerContext.tsx

… to check signal and code every time

ravicious

Looking good. 👍

I have a feeling we might have missed some edge cases, but it'll be much easier to iron them out with the full lifecycle being controllable from the UI.

web/packages/teleterm/package.json

...m/src/ui/ConnectMyComputer/DocumentConnectMyComputerSetup/DocumentConnectMyComputerSetup.tsx

gzdunek · 2023-08-02T15:59:33Z

Friendly ping @avatus

avatus

I only did a code review and didn't find anything both of you didn't already discuss. I didn't get a chance to try it locally, sorry.

Very impressive stuff tho, the quality is top notch imo (as usual)

espadolini

bot!

gzdunek requested a review from ravicious July 24, 2023 17:01

gzdunek force-pushed the gzdunek/cmc-run-agent branch from e054576 to 7628d06 Compare July 24, 2023 17:02

gzdunek changed the title ~~Connect My Computer: Run agent~~ Connect My Computer: Join cluster Jul 24, 2023

gzdunek force-pushed the gzdunek/cmc-run-agent branch from 7628d06 to 365a5c4 Compare July 24, 2023 17:07

ravicious reviewed Jul 25, 2023

View reviewed changes

Base automatically changed from gzdunek/cmc-agent-config-file to master July 25, 2023 16:51

gzdunek added 5 commits July 25, 2023 18:56

Add generateAgentConfigPaths function that creates config path base…

5376e1a

…d on `runtimeSettings` and `rootClusterUri`. Pass `rootClusterUri` instead of `profileName` `createAgentFile`

Add functions to run agent and subscribe to its events

ca6a5b2

Clear attempts when restarting the process

da19a68

Run the agent from the UI and remove node token

ea2c34f

Show errors from the process in the setup UI

8da756d

gzdunek mentioned this pull request Jul 25, 2023

Connect My Computer #27881

Closed

8 tasks

gzdunek force-pushed the gzdunek/cmc-run-agent branch from 365a5c4 to 7ad0e97 Compare July 26, 2023 14:04

gzdunek requested a review from avatus July 26, 2023 14:06

gzdunek marked this pull request as ready for review July 26, 2023 14:06

github-actions bot added size/lg ui labels Jul 26, 2023

github-actions bot requested review from kimlisa and ravicious July 26, 2023 14:07

gzdunek removed the request for review from kimlisa July 26, 2023 14:07

gzdunek added 8 commits July 26, 2023 16:09

Refactor reporting errors from the agent process

904e253

Add isLocalBuild

ff265fa

Join arguments with space when logging

7ba35c4

Add killProcess function that handles process closing

40a6231

Spawn a real process in agentRunner tests

1e3a5bd

Keep agentRunner files in a single directory

b32f405

Catch errors from deleteToken

b94bd7e

Remove env: process.env

72eaf48

gzdunek force-pushed the gzdunek/cmc-run-agent branch from 7ad0e97 to 72eaf48 Compare July 26, 2023 14:15

ravicious reviewed Jul 28, 2023

View reviewed changes

gzdunek added 7 commits July 28, 2023 15:30

Reject when an agent process fails to start in test

a1ca16f

Match only on "ENOENT"

cdc0e8f

Correct test name ("SIGTERM" -> "SIGKILL")

00234d7

Test terminating the process and then trying to kill it

ec73591

Wait for "exit" event instead of "close"

0a6a5e6

Rename killProcess to terminateWithTimeout

103483b

Add getAgentState method to synchronously get the agent state

53e2cc5

ravicious reviewed Jul 31, 2023

View reviewed changes

ravicious self-requested a review July 31, 2023 15:02

gzdunek added 4 commits July 31, 2023 17:24

Remove space before new line

040a50f

Simplify the logic in AgentRunner

ecdf232

Fix TS error

086bc16

ravicious reviewed Aug 1, 2023

View reviewed changes

gzdunek added 5 commits August 1, 2023 17:36

Do not send agent updates to a destroyed window

0eebf95

Add logging cluster URI and updated state

aef7d7e

Catch errors that are thrown while spawning the process

d3b858a

Strip ANSI codes

a57477a

Add exitedSuccessfully property to exited state, so we won't have…

d80094d

… to check signal and code every time

ravicious approved these changes Aug 2, 2023

View reviewed changes

web/packages/teleterm/package.json Outdated Show resolved Hide resolved

...m/src/ui/ConnectMyComputer/DocumentConnectMyComputerSetup/DocumentConnectMyComputerSetup.tsx Show resolved Hide resolved

gzdunek added 2 commits August 2, 2023 16:30

Move strip-ansi-stream to dependencies

0b55dfd

Fix license

5a466e2

avatus approved these changes Aug 2, 2023

View reviewed changes

espadolini approved these changes Aug 3, 2023

View reviewed changes

gzdunek added this pull request to the merge queue Aug 3, 2023

Merged via the queue into master with commit cc681c9 Aug 3, 2023

gzdunek deleted the gzdunek/cmc-run-agent branch August 3, 2023 09:44

ravicious mentioned this pull request Aug 24, 2023

Improve agent log processing (no cut off logs + close event instead of exit) #30941

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Connect My Computer: Join cluster #29479

Connect My Computer: Join cluster #29479

gzdunek commented Jul 24, 2023

ravicious left a comment

ravicious left a comment

ravicious left a comment

ravicious left a comment

gzdunek commented Aug 2, 2023

avatus left a comment

espadolini left a comment

Connect My Computer: Join cluster #29479

Connect My Computer: Join cluster #29479

Conversation

gzdunek commented Jul 24, 2023

ravicious left a comment

Choose a reason for hiding this comment

ravicious left a comment

Choose a reason for hiding this comment

ravicious left a comment

Choose a reason for hiding this comment

ravicious left a comment

Choose a reason for hiding this comment

gzdunek commented Aug 2, 2023

avatus left a comment

Choose a reason for hiding this comment

espadolini left a comment

Choose a reason for hiding this comment